-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ensure_async should not silently eat errors and return the coro instead of the result #138
base: main
Are you sure you want to change the base?
Conversation
@@ -81,13 +81,6 @@ async def ensure_async(obj: Union[Awaitable, Any]) -> Any: | |||
and await it if it was not already awaited. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring has to be changed too, since we always await if the object is a coroutine.
The code you removed was here to handle the error that you see in the tests: |
But in this case |
No, because the exception raised was |
So this means that |
Almost :), inside |
I agree that the code you removed in this PR should not have been there, and it was mostly to fix an issue that I didn't fully understand. We should probably fix |
Yeah, I don't understand why it happens, I've taken a quick look, but
cannot find the issue. I hope you have some idea.
(from mobile phone)
…On Fri, Mar 5, 2021, 17:24 David Brochart ***@***.***> wrote:
I agree that the code you removed in this PR should not have been there,
and it was mostly to fix an issue that I didn't fully understand. We should
probably fix is_alive() as you mentioned, because it shouldn't return
twice the same coroutine.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#138 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANPEPNMZ7JUPVJCWYMX4TTTCEASZANCNFSM4YVE5JUA>
.
|
I've looked too, but jupyter-client is becoming really complicated. I opened jupyter/jupyter_client#621. |
We are now returning the coroutine itself, instead of the results of the coroutine.
This gets triggered when a coroutine that is wrapped actually awaits the same coroutine twice. In my case in voila, this caused kernel_id https://github.com/voila-dashboards/voila/blob/4b323460fd5d7e6a7203ff0492607c13c8eb6f25/voila/handler.py#L166 to be assigned the coroutine, because the
start_kernel
had an exception.Note that this change causes the tests to fail, I'm not sure how to fix this.